From 7eb5ea543d0261b7d0f80fd83275d1590f7df12e Mon Sep 17 00:00:00 2001 From: Yehuda Katz Date: Thu, 10 Jul 2014 00:27:40 -0700 Subject: [PATCH] Fixed a number of bugs related to rustc building Most notably, `resolve` now takes the root, so it can properly link the root package with its dependencies (which is required to build the --externs for the root package). --- src/cargo/core/manifest.rs | 15 ++++++++ src/cargo/core/resolver.rs | 63 ++++++++++++++++------------------ src/cargo/ops/cargo_compile.rs | 5 +-- src/cargo/ops/cargo_rustc.rs | 48 +++++++++++++++++++++++--- src/cargo/util/graph.rs | 4 +++ tests/test_cargo_compile.rs | 3 +- 6 files changed, 97 insertions(+), 41 deletions(-) diff --git a/src/cargo/core/manifest.rs b/src/cargo/core/manifest.rs index 4871815d7..9d4da5c2a 100644 --- a/src/cargo/core/manifest.rs +++ b/src/cargo/core/manifest.rs @@ -344,6 +344,21 @@ impl Target { } } + pub fn is_dylib(&self) -> bool { + match self.kind { + LibTarget(ref kinds) => kinds.iter().any(|&k| k == Dylib), + _ => false + } + } + + pub fn is_rlib(&self) -> bool { + match self.kind { + LibTarget(ref kinds) => + kinds.iter().any(|&k| k == Rlib || k == Lib), + _ => false + } + } + pub fn is_bin(&self) -> bool { match self.kind { BinTarget => true, diff --git a/src/cargo/core/resolver.rs b/src/cargo/core/resolver.rs index 607cee781..27e2631c0 100644 --- a/src/cargo/core/resolver.rs +++ b/src/cargo/core/resolver.rs @@ -4,7 +4,6 @@ use util::graph::{Nodes,Edges}; use core::{ Dependency, PackageId, - Summary, Registry, SourceId, }; @@ -31,12 +30,6 @@ impl Resolve { } } -/* -pub fn resolve(deps: &[Dependency], registry: &mut R) -> CargoResult> { - Ok(try!(resolve2(deps, registry)).iter().map(|p| p.clone()).collect()) -} -*/ - struct Context<'a, R> { registry: &'a mut R, resolve: Resolve, @@ -57,15 +50,17 @@ impl<'a, R: Registry> Context<'a, R> { } } -pub fn resolve(deps: &[Dependency], registry: &mut R) -> CargoResult { +pub fn resolve(root: &PackageId, deps: &[Dependency], registry: &mut R) + -> CargoResult +{ log!(5, "resolve; deps={}", deps); let mut context = Context::new(registry); - try!(resolve_deps(None, deps, &mut context)); + try!(resolve_deps(root, deps, &mut context)); Ok(context.resolve) } -fn resolve_deps<'a, R: Registry>(parent: Option<&PackageId>, +fn resolve_deps<'a, R: Registry>(parent: &PackageId, deps: &[Dependency], ctx: &mut Context<'a, R>) -> CargoResult<()> @@ -91,6 +86,8 @@ fn resolve_deps<'a, R: Registry>(parent: Option<&PackageId>, let source_id = summary.get_source_id().clone(); let version = summary.get_version().clone(); + ctx.resolve.graph.link(parent.clone(), summary.get_package_id().clone()); + let found = { let found = ctx.seen.find(&(name.clone(), source_id.clone())); @@ -115,11 +112,7 @@ fn resolve_deps<'a, R: Registry>(parent: Option<&PackageId>, .map(|d| d.clone()) .collect(); - try!(resolve_deps(Some(summary.get_package_id()), deps.as_slice(), ctx)); - - parent.map(|parent| { - ctx.resolve.graph.link(parent.clone(), summary.get_package_id().clone()); - }); + try!(resolve_deps(summary.get_package_id(), deps.as_slice(), ctx)); } Ok(()) @@ -131,11 +124,12 @@ mod test { use core::source::{SourceId, RegistryKind, Location, Remote}; use core::{Dependency, PackageId, Summary, Registry}; - use super::resolve; use util::CargoResult; - fn resolve(deps: &[Dependency], registry: &mut R) -> CargoResult> { - Ok(try!(super::resolve(deps, registry)).iter().map(|p| p.clone()).collect()) + fn resolve(pkg: &PackageId, deps: &[Dependency], registry: &mut R) + -> CargoResult> + { + Ok(try!(super::resolve(pkg, deps, registry)).iter().map(|p| p.clone()).collect()) } trait ToDep { @@ -176,8 +170,11 @@ mod test { } fn pkg(name: &str) -> Summary { - Summary::new(&PackageId::new(name, "1.0.0", ®istry_loc()).unwrap(), - &[]) + Summary::new(&pkg_id(name), &[]) + } + + fn pkg_id(name: &str) -> PackageId { + PackageId::new(name, "1.0.0", ®istry_loc()).unwrap() } fn dep(name: &str) -> Dependency { @@ -198,7 +195,7 @@ mod test { #[test] pub fn test_resolving_empty_dependency_list() { - let res = resolve([], &mut registry(vec!())).unwrap(); + let res = resolve(&pkg_id("root"), [], &mut registry(vec!())).unwrap(); assert_that(&res, equal_to(&names([]))); } @@ -206,41 +203,41 @@ mod test { #[test] pub fn test_resolving_only_package() { let mut reg = registry(vec!(pkg("foo"))); - let res = resolve([dep("foo")], &mut reg); + let res = resolve(&pkg_id("root"), [dep("foo")], &mut reg); - assert_that(&res.unwrap(), equal_to(&names(["foo"]))); + assert_that(&res.unwrap(), contains(names(["root", "foo"])).exactly()); } #[test] pub fn test_resolving_one_dep() { let mut reg = registry(vec!(pkg("foo"), pkg("bar"))); - let res = resolve([dep("foo")], &mut reg); + let res = resolve(&pkg_id("root"), [dep("foo")], &mut reg); - assert_that(&res.unwrap(), equal_to(&names(["foo"]))); + assert_that(&res.unwrap(), contains(names(["root", "foo"])).exactly()); } #[test] pub fn test_resolving_multiple_deps() { let mut reg = registry(vec!(pkg!("foo"), pkg!("bar"), pkg!("baz"))); - let res = resolve([dep("foo"), dep("baz")], &mut reg).unwrap(); + let res = resolve(&pkg_id("root"), [dep("foo"), dep("baz")], &mut reg).unwrap(); - assert_that(&res, contains(names(["foo", "baz"])).exactly()); + assert_that(&res, contains(names(["root", "foo", "baz"])).exactly()); } #[test] pub fn test_resolving_transitive_deps() { let mut reg = registry(vec!(pkg!("foo"), pkg!("bar" => "foo"))); - let res = resolve([dep("bar")], &mut reg).unwrap(); + let res = resolve(&pkg_id("root"), [dep("bar")], &mut reg).unwrap(); - assert_that(&res, contains(names(["foo", "bar"]))); + assert_that(&res, contains(names(["root", "foo", "bar"]))); } #[test] pub fn test_resolving_common_transitive_deps() { let mut reg = registry(vec!(pkg!("foo" => "bar"), pkg!("bar"))); - let res = resolve([dep("foo"), dep("bar")], &mut reg).unwrap(); + let res = resolve(&pkg_id("root"), [dep("foo"), dep("bar")], &mut reg).unwrap(); - assert_that(&res, contains(names(["foo", "bar"]))); + assert_that(&res, contains(names(["root", "foo", "bar"]))); } #[test] @@ -252,8 +249,8 @@ mod test { pkg!("bat") )); - let res = resolve([dep("foo"), dep("baz").as_dev()], &mut reg).unwrap(); + let res = resolve(&pkg_id("root"), [dep("foo"), dep("baz").as_dev()], &mut reg).unwrap(); - assert_that(&res, contains(names(["foo", "bar", "baz"]))); + assert_that(&res, contains(names(["root", "foo", "bar", "baz"]))); } } diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index aae4c0d62..340ff4c26 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -63,8 +63,9 @@ pub fn compile(manifest_path: &Path, options: CompileOptions) -> CargoResult<()> let mut registry = try!(PackageRegistry::new(source_ids, override_ids, &mut config)); - let resolved = - try!(resolver::resolve(package.get_dependencies(), &mut registry)); + let resolved = try!(resolver::resolve(package.get_package_id(), + package.get_dependencies(), + &mut registry)); let req: Vec = resolved.iter().map(|r| r.clone()).collect(); let packages = try!(registry.get(req.as_slice()).wrap({ diff --git a/src/cargo/ops/cargo_rustc.rs b/src/cargo/ops/cargo_rustc.rs index 7764a8858..4ce26e1c6 100644 --- a/src/cargo/ops/cargo_rustc.rs +++ b/src/cargo/ops/cargo_rustc.rs @@ -4,11 +4,12 @@ use std::io::{File, IoError}; use std::io; use std::os::args; use std::str; +use std::io::TempDir; use term::color::YELLOW; use core::{Package, PackageSet, Target, Resolve}; use util; -use util::{CargoResult, ChainError, ProcessBuilder, internal, human, CargoError}; +use util::{CargoResult, ChainError, ProcessBuilder, CargoError, Require, internal, human}; use util::{Config, TaskPool, DependencyQueue, Fresh, Dirty, Freshness}; type Args = Vec; @@ -20,7 +21,8 @@ struct Context<'a, 'b> { rustc_version: &'a str, resolve: &'a Resolve, package_set: &'a PackageSet, - config: &'b mut Config<'b> + config: &'b mut Config<'b>, + dylib: (String, String) } type Job = proc():Send -> CargoResult<()>; @@ -71,6 +73,24 @@ pub fn compile_targets<'a>(env: &str, targets: &[&Target], pkg: &Package, internal(format!("Couldn't create the directory for dependencies for {} at {}", pkg.get_name(), deps_target_dir.display())))); + let temp: TempDir = try!(TempDir::new_in(&deps_target_dir, "temp") + .require(|| internal("Couldn't create a tempdir"))); + + let file = temp.path().join("dylib.rs"); + try!(File::create(&file)); + + let output = try!(util::process("rustc") + .arg(file.display().to_str()) + .arg("--crate-name").arg("-") + .arg("--crate-type").arg("dylib") + .arg("--print-file-name") + .exec_with_output()); + + let output = str::from_utf8(output.output.as_slice()).unwrap(); + + let parts: Vec<&str> = output.slice_to(output.len() - 1).split('-').collect(); + assert!(parts.len() == 2, "rustc --print-file-name output has changed"); + let mut cx = Context { dest: &deps_target_dir, deps_dir: &deps_target_dir, @@ -78,7 +98,8 @@ pub fn compile_targets<'a>(env: &str, targets: &[&Target], pkg: &Package, rustc_version: rustc_version.as_slice(), resolve: resolve, package_set: deps, - config: config + config: config, + dylib: (parts.get(0).to_str(), parts.get(1).to_str()) }; // Build up a list of pending jobs, each of which represent compiling a @@ -87,6 +108,8 @@ pub fn compile_targets<'a>(env: &str, targets: &[&Target], pkg: &Package, // everything in order with proper parallelism. let mut jobs = Vec::new(); for dep in deps.iter() { + if dep == pkg { continue; } + // Only compile lib targets for dependencies let targets = dep.get_targets().iter().filter(|target| { target.is_lib() && target.get_profile().get_env() == env @@ -228,8 +251,10 @@ fn rustc(package: &Package, target: &Target, cx: &mut Context) -> Job { proc() { if primary { + log!(5, "executing primary"); rustc.exec().map_err(|err| human(err.to_str())) } else { + log!(5, "executing deps"); rustc.exec_with_output().and(Ok(())).map_err(|err| { human(err.to_str()) }) @@ -315,10 +340,23 @@ fn build_deps_args(dst: &mut Args, package: &Package, cx: &Context) { for target in dep_targets(package, cx).iter() { dst.push("--extern".to_str()); - dst.push(format!("{}={}/lib{}.rlib", + dst.push(format!("{}={}/{}", target.get_name(), cx.deps_dir.display(), - target.file_stem())); + target_filename(target, cx))); + } +} + +fn target_filename(target: &Target, cx: &Context) -> String { + let stem = target.file_stem(); + + if target.is_dylib() { + let (ref prefix, ref suffix) = cx.dylib; + format!("{}{}{}", prefix, stem, suffix) + } else if target.is_rlib() { + format!("lib{}.rlib", stem) + } else { + unreachable!() } } diff --git a/src/cargo/util/graph.rs b/src/cargo/util/graph.rs index e8c57f339..cf96c2d47 100644 --- a/src/cargo/util/graph.rs +++ b/src/cargo/util/graph.rs @@ -29,6 +29,10 @@ impl Graph { .insert(child); } + pub fn get_nodes<'a>(&'a self) -> &'a HashMap> { + &self.nodes + } + pub fn edges<'a>(&'a self, node: &N) -> Option> { self.nodes.find(node).map(|set| set.iter()) } diff --git a/tests/test_cargo_compile.rs b/tests/test_cargo_compile.rs index 1c91da209..a30483125 100644 --- a/tests/test_cargo_compile.rs +++ b/tests/test_cargo_compile.rs @@ -1061,7 +1061,8 @@ test!(verbose_release_build_deps { -C extra-filename={hash2} \ --out-dir {dir}{sep}target{sep}release \ -L {dir}{sep}target{sep}release \ - -L {dir}{sep}target{sep}release{sep}deps` + -L {dir}{sep}target{sep}release{sep}deps \ + --extern foo={dir}/target/release/deps/libfoo-12f497cde367dfe6.rlib` {compiling} foo v0.0.0 (file:{dir}) {compiling} test v0.0.0 (file:{dir})\n", running = RUNNING, -- 2.30.2